Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Housekeeping #277

Merged
merged 4 commits into from
Dec 11, 2024
Merged

Housekeeping #277

merged 4 commits into from
Dec 11, 2024

Conversation

avylove
Copy link
Collaborator

@avylove avylove commented Nov 23, 2024

A few housekeeping tasks

  • Fix pylint errors that were breaking CI tests
    • Too many positional arguments on Termcap.build()
  • Fix Sphinx issues that were breaking CI tests
    • Deprecated behavior in conf.py
    • csv-table used for keycodes now requires the deliminator in the header rather than a comma
  • Update Python versions
    • Default for testing switched to 3.13
    • Added 3.14-pre as optional in CI
  • Added logic to CI to use Codecov uploader version 4 except for Python 2.7, which uses version 3
    • Maybe this will help with the random rate limit errors
    • They released version 5 on November 14th, but it might be good to wait since they have done 7 bug releases since then

Copy link

codecov bot commented Nov 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.31%. Comparing base (167c34e) to head (57f6d5f).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #277   +/-   ##
=======================================
  Coverage   95.31%   95.31%           
=======================================
  Files           9        9           
  Lines        1025     1025           
  Branches      216      177   -39     
=======================================
  Hits          977      977           
  Misses         44       44           
  Partials        4        4           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@@ -155,7 +154,7 @@ def wrapper(decorator):
# html_theme_options = {}

# Add any paths that contain custom themes here, relative to this directory.
html_theme_path = [sphinx_rtd_theme.get_html_theme_path()]
# html_theme_path = []
Copy link
Owner

@jquast jquast Dec 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't mention why to remove this, I think I put it here to match local development to the theme where it is published, in any case it uses a requirement "sphinx_rtd_theme" in docs/requirements.txt, so then that requirement should also be removed.

Sorry to be late, I've been busy with school work but that's all over tomorrow

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's because of this warning from Sphinx. We are treating warnings as errors.

WARNING: Calling get_html_theme_path is deprecated. If you are calling it to define html_theme_path, you are safe to remove that code.

But I think we can leave the dependency because we still have
html_theme = "sphinx_rtd_theme"

I think back in the day the theme loading needed extra help that isn't required anymore. But building locally, with the changes, the docs look the same as on Read the Docs.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, great! Thanks for figuring that out!

@jquast jquast self-requested a review December 10, 2024 21:44
Copy link
Owner

@jquast jquast left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

approved -- but maybe you want to delete sphinx_rtd_theme requirement ?

@avylove avylove merged commit 9bc89e6 into master Dec 11, 2024
29 checks passed
@avylove avylove deleted the codecov_tweaks branch December 11, 2024 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants